Skip to content

Conversation

@BigTava
Copy link
Contributor

@BigTava BigTava commented Jan 6, 2025

Reason for the proposed changes

Please describe what we want to achieve and why.

Proposed changes

INTEGRATION_TESTS_BRANCH=master
GSY_FRAMEWORK_BRANCH=feature/GSYE-719

out_key = results_field_to_json_filename_mapping[in_key]

# Only to export carbon emissions
if in_key == "trade_profile" and self.country_code:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be trade_profile?

Copy link
Member

@hannesdiedrich hannesdiedrich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two comments, will review again after adapted.

"--country-code",
type=str,
default=None,
help="Country code according to ISO 3166-1 alpha-2.",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO a small sentence would be valuable that explains that this option is only needed for the carbon footprint calculation. If not provided, no carbon footprint is extracted.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

simulation.run()

# Then
print("results = ", simulation._results._endpoint_buffer.generate_json_report())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, nothing is tested.
Additionally, this is an integration test that runs a simulation. We should not run a simulation only for testing if a result file is extracted here. We have an integration test scenario that checks for the export of the files:
"Test aggregated results are exported" You could adapt this one for a kind of e2e test.

The correct way for a unit test would be to check if the carbon_emissions_handler. calculate_from_gsy_imported_exported_energy is called by the ExportAndPlot class under the right circumstances.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants